Conversation
jlbrooks
left a comment
jlbrooks
left a comment
There was a problem hiding this comment.
Overall looks pretty good, just a couple minor cleanup changes and notes about extension points.
| volatile uint8_t tx_buffer[TX_BUFFER_SIZE]; | ||
|
|
||
| void lora_setup(void); | ||
| void lora_join(void); |
There was a problem hiding this comment.
Void returns for most of these functions are fine, but since join has a variety of outcomes this should probably communicate some more information back to the calling function. You could define a lora_error_t struct or similar, or just change it to an int and #define some return values.
| */ | ||
|
|
||
| #include <stdint.h> | ||
| #include <stdbool.h> |
There was a problem hiding this comment.
Nitpick, these can live inside the #IFNDEF
| void printSerial(uint8_t cmd[],uint16_t len); | ||
| bool read_response(void); | ||
| void send_lora_count(uint16_t count); | ||
|
|
There was a problem hiding this comment.
Not necessary for this PR, but we do need to start looking at the LoRaWAN activation by personalization so that we don't need to do OTAA join every time. I added a Trello card and assigned you to it to track.
There was a problem hiding this comment.
In a similar vein, we should start investigating RX and determine if that'll be possible. We will want to be able to receive for OTA configuration.
| config_usart.pinmux_pad0 = EDBG_CDC_SERCOM_PINMUX_PAD0; | ||
| config_usart.pinmux_pad1 = EDBG_CDC_SERCOM_PINMUX_PAD1; | ||
| config_usart.pinmux_pad2 = EDBG_CDC_SERCOM_PINMUX_PAD2; | ||
| config_usart.pinmux_pad3 = EDBG_CDC_SERCOM_PINMUX_PAD3; |
There was a problem hiding this comment.
All of the serial debug stuff should probably be wrapped inside of #IFDEF's. We don't want to be writing debug output if not necessary.
2 participants
Corrected PR. Only 2 new files - loraDriver.c and h